fix(cache,da): drop finalized snapshot entries on restore and drain DA subscription channel by tac0turtle · Pull Request #3385 · evstack/ev-node

tac0turtle · 2026-07-15T13:24:43Z

Overview

Two memory fixes for long-running nodes, found while investigating a user report of sustained memory growth (+201 MiB/min) and OOM kills on syncing nodes.

1. Cache restore leaks stale placeholders across crash/restart cycles

The DA-inclusion snapshot is only persisted on graceful shutdown (Components.Stop). After a crash (e.g. OOM kill, SIGKILL), the node restores a snapshot from the last graceful shutdown, which can contain heights already at or below the persisted DAIncludedHeight watermark. Since:

the inclusion loop only calls DeleteHeight for heights it advances through, and
IsHeightDAIncluded short-circuits true below the watermark so those entries are never consulted,

restored placeholders below the watermark were never evicted, and were re-persisted on the next graceful save — so the snapshot grew monotonically across crash/restart cycles.

Cache.RestoreFromStore now takes the finalized height and skips entries at or below it. Skipped entries still contribute to maxDAHeight, so DaHeight() semantics are unchanged. manager.RestoreFromStore reads DAIncludedHeightKey and passes it through; a read error falls back to unfiltered restore instead of failing startup.

The pre-existing comment in TestManager_SaveAndRestoreFromStore already described this as the intended behavior ("The cache entry is not restored — this is correct and intentional") — the code just didn't do it. It now does, and the test asserts it.

2. DA subscription goroutine leak on reconnect

The client.Subscribe wrapper goroutine exited on ctx cancellation without draining the underlying jsonrpc channel. The go-jsonrpc delivery goroutine blocked on send into that channel never observed the cancellation and never closed its channel — leaking one goroutine per watchdog reconnect (observed in the field as Subscribe.func1 goroutines doubling). The wrapper now drains the raw channel on exit, matching what its own doc comment previously required of callers.

Testing

New TestCache_RestoreFromStore_SkipsFinalizedEntries covering partial and fully-stale snapshots
TestManager_SaveAndRestoreFromStore now asserts finalized heights are not restored
go test ./block/..., go vet, golangci-lint all clean

Note: this does not address the primary OOM driver from the same investigation (unbounded pendingEvents buffering when DA catchup outpaces execution) — that needs a separate backpressure change.

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Prevented finalized cache entries from being restored as active placeholders after restart.
- Preserved the correct maximum data-availability height while removing obsolete entries.
- Improved cache restoration when persisted metadata is unavailable.
- Prevented subscription shutdown and reconnection scenarios from leaving message delivery blocked.
Reliability
- Improved consistency of restored header and data availability state across restarts.

…A subscription channel Two memory fixes for long-running nodes: Cache restore: the DA-inclusion snapshot is only written on graceful shutdown, so after a crash (e.g. OOM kill) the restored snapshot can contain heights already below the persisted DA-included watermark. The inclusion loop never evicts below its watermark, so those placeholder entries leaked for the process lifetime and were re-persisted on every subsequent save, growing the snapshot monotonically across crash/restart cycles. RestoreFromStore now skips entries at or below the persisted DAIncludedHeight; skipped entries still seed maxDAHeight so DaHeight() is unchanged. DA subscription: the Subscribe wrapper goroutine exited on ctx cancellation without draining the underlying jsonrpc channel, leaving the go-jsonrpc delivery goroutine blocked on send — it never observed the cancellation and never closed its channel, leaking one goroutine per watchdog reconnect. The wrapper now drains the raw channel on exit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-07-15T13:24:53Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ee127894-6385-4068-92d9-4d2930c23502

📥 Commits

Reviewing files that changed from the base of the PR and between 19d3a86 and 896871f.

📒 Files selected for processing (5)

block/internal/cache/generic_cache.go
block/internal/cache/generic_cache_test.go
block/internal/cache/manager.go
block/internal/cache/manager_test.go
block/internal/da/client.go

📝 Walkthrough

Walkthrough

Cache restoration now accepts a finalized-height watermark, skips finalized placeholders while retaining DA height, and passes persisted metadata through manager restoration. DA subscription shutdown now drains the underlying JSON-RPC channel.

Changes

Watermarked cache restoration

Layer / File(s)	Summary
Finalized-aware snapshot restoration `block/internal/cache/generic_cache.go`, `block/internal/cache/generic_cache_test.go`	`RestoreFromStore` preserves the maximum decoded DA height while skipping snapshot entries at or below the finalized height; tests cover stale and partially finalized snapshots.
Manager integration and regression coverage `block/internal/cache/manager.go`, `block/internal/cache/manager_test.go`, `block/internal/cache/generic_cache_test.go`	Manager restoration reads the persisted DA-included height and passes it to both caches; restore tests use the updated signature and verify finalized entries are absent.

DA subscription shutdown

Layer / File(s)	Summary
Subscription channel draining `block/internal/da/client.go`	`Subscribe` drains the raw JSON-RPC channel in a deferred shutdown loop and updates its lifecycle documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CacheManager
  participant Store
  participant Cache
  CacheManager->>Store: read DAIncludedHeightKey
  CacheManager->>Cache: restore with finalized height
  Cache->>Cache: update DA height from snapshot
  Cache->>Cache: install only non-finalized placeholders

sequenceDiagram
  participant Caller
  participant Subscribe
  participant JSONRPC
  Caller->>Subscribe: cancel context
  Subscribe->>JSONRPC: drain raw channel
  JSONRPC-->>Subscribe: close channel

Possibly related PRs

evstack/ev-node#3204: Also changes cache snapshot restoration and storage behavior.

Suggested reviewers: julienrbrt

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes both main fixes: skipping finalized cache entries on restore and draining the DA subscription channel.
Description check	✅ Passed	The description matches the template with a clear Overview, rationale, and testing section, and it explains both fixes in enough detail.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch marko/cache-restore-filter-sub-drain

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

github-actions · 2026-07-15T13:25:03Z

The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
`✅ passed`	`⏩ skipped`	`✅ passed`	`⏩ skipped`	Jul 16, 2026, 10:29 AM

codecov · 2026-07-15T13:46:09Z

Codecov Report

❌ Patch coverage is 50.00000% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.32%. Comparing base (19d3a86) to head (896871f).

Files with missing lines	Patch %	Lines
block/internal/cache/manager.go	16.66%	2 Missing and 3 partials ⚠️
block/internal/da/client.go	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3385      +/-   ##
==========================================
- Coverage   62.33%   62.32%   -0.01%     
==========================================
  Files         120      120              
  Lines       13444    13452       +8     
==========================================
+ Hits         8380     8384       +4     
- Misses       4124     4126       +2     
- Partials      940      942       +2

Flag	Coverage Δ
combined	`62.32% <50.00%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions Bot assigned tac0turtle Jul 15, 2026

Merge branch 'main' into marko/cache-restore-filter-sub-drain

b769abd

Merge branch 'main' into marko/cache-restore-filter-sub-drain

896871f

tac0turtle merged commit b4b895b into main Jul 16, 2026
28 checks passed

tac0turtle deleted the marko/cache-restore-filter-sub-drain branch July 16, 2026 10:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cache,da): drop finalized snapshot entries on restore and drain DA subscription channel#3385

fix(cache,da): drop finalized snapshot entries on restore and drain DA subscription channel#3385
tac0turtle merged 3 commits into
mainfrom
marko/cache-restore-filter-sub-drain

tac0turtle commented Jul 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 15, 2026 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Jul 15, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Jul 15, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

tac0turtle commented Jul 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

1. Cache restore leaks stale placeholders across crash/restart cycles

2. DA subscription goroutine leak on reconnect

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

github-actions Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jul 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

tac0turtle commented Jul 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 15, 2026 •

edited

Loading

github-actions Bot commented Jul 15, 2026 •

edited

Loading

codecov Bot commented Jul 15, 2026 •

edited

Loading